Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

VirtualBox: enable ability to import b2d instances #979

Merged
merged 7 commits into from
Apr 23, 2015

Conversation

ehazlett
Copy link
Contributor

@ehazlett ehazlett commented Apr 8, 2015

This adds support for importing a b2d instance (in this case, just the disk).

Discussion: should we do a full clone of the VM to bring in any changes the user may make to the VM itself? I didn't do that in the current state as this might add things that Machine does not support (i.e. network configuration, etc).

Refs #904

@nathanleclaire
Copy link
Contributor

Cool. That was fast!

If we're going to do the migration this way, we will need:

  1. Documentation on how to do it (I don't think most users will know that they need to specify by name boot2docker-vm)
  2. Game-plan for deprecating this feature eventually (2 releases? 3?)

Discussion: should we do a full clone of the VM to bring in any changes the user may make to the VM itself? I didn't do that in the current state as this might add things that Machine does not support (i.e. network configuration, etc).

I'm not too worried about importing settings that the user has made manually, since that seems to me that it could be a large amount of work for a not so common use case, but I do think we need to parse the b2d config file and convert it into machine terms. If my b2d configuration was set to allow 4096MB RAM and the new machine doesn't have the same amount, that's going to be an annoying surprise.

@ehazlett
Copy link
Contributor Author

ehazlett commented Apr 8, 2015

@nathanleclaire thanks! yeah i need to do a separate guideline -- i think that should be a separate issue / PR -- thoughts?

+1 for planning for the deprecation

@ehazlett
Copy link
Contributor Author

added the ability to import cpu and memory settings from the b2d vm. PTAL @nathanleclaire

return err
}

if err := os.Chmod(dst, fi.Mode()); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this solving? I'm not doubting that it's needed, just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you copy a key it will not have the correct permissions. When you then try to ssh, it will fail because we shell out to ssh which the file must be 0600. Here I copy the original file permissions to match the existing key perms.

@nathanleclaire
Copy link
Contributor

Code is looking good, modulo a few nitpicks- I'll give this functionality a whirl soon.

@ehazlett
Copy link
Contributor Author

@nathanleclaire updated ptal

cli.StringFlag{
Name: "virtualbox-import-boot2docker-vm",
Usage: "The name of a Boot2Docker VM to import",
Value: "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep thinking that maybe we should default this value to boot2docker-vm, since that's by far the most common name and I have a feeling users will run this flag without args expecting it to "just work".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, I'd certainly expect that myself -- I don't know that's it's very common for users to actually name their boot2docker VMs (even though it's supported to do so)

@nathanleclaire
Copy link
Contributor

So should we clean up the old boot2docker-vm automatically or prompt users to boot2docker destroy?

@nathanleclaire
Copy link
Contributor

I added an issue to write docs for the 0.3.0 milestone. Tried this on the Windows box I have and it worked well.

LGTM

@ehazlett
Copy link
Contributor Author

@nathanleclaire Yeah I think that would be in the docs. I don't want to remove it automatically -- that seems too destructive (especially if the import fails for some reason). I think a note in the migration doc would suffice. Thoughts?

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@ehazlett
Copy link
Contributor Author

@nathanleclaire added docs -- this should close #1047. PTAL

@nathanleclaire
Copy link
Contributor

Yeah I think that would be in the docs. I don't want to remove it automatically -- that seems too destructive (especially if the import fails for some reason). I think a note in the migration doc would suffice. Thoughts?

Recommending boot2docker destroy (and maybe an uninstall of boot2docker as well) seems reasonable to me.

ehazlett added a commit that referenced this pull request Apr 23, 2015
VirtualBox: enable ability to import b2d instances
@ehazlett ehazlett merged commit 3f4ce28 into docker:master Apr 23, 2015
@ehazlett ehazlett deleted the b2d-import-disk branch April 23, 2015 19:09
@nathanleclaire
Copy link
Contributor

👍 👍 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants